The diag implementation looks good.

It's a useful method, explain() would also be really useful, i'd use that one more.

Thanks for the very detailed pull request :)

@Raynos Implementing explain() should be trivial. There is just one detail to resolve first: How exactly should one put tape into verbose mode when running in the browser? On the command line this can be trivially accomplished with a --verbose CLI flag. In a server-side process you can use a command line flag when starting the process or use ENV variables. With a browser side test, this isn't so easy since you may not have any real control over how the runtime gets executed/setup or what global context is available at runtime. The best option I can think of is setting a global flag that is set by your build system when in concatenates your JS files when dynamically generating JS scripts.

Also, should explain() be a binary on/off feature, or should it support some sort of log levels feature. e.g. only show me logging information of type performance or with importance above a level of 40

@Ovid any thoughts on diagnostic output levels versus a simple on/off switch?

I don't think explain() needs anything akin to log levels primarily because I use it when I'm writing new tests and implementing a feature. When I do that, I want to see those explain messages — all of them. Thus, for me it really is a boolean on/off.

That being said, maybe someone else has a different use case I'm unaware. I teach a lot of people how to test and I haven't seen a different use case, but it doesn't mean it doesn't exist.
 
Best,

Ovid

IT consulting, training, international recruiting
       http://www.allaroundtheworld.fr/.
Buy my book! - http://bit.ly/beginning_perl
Live and work overseas - http://www.overseas-exile.com/

On Friday, 15 August 2014, 12:17, Andrew de Andrade [email protected] wrote:

@Raynos Implementing explain() should be trivial. There is just one detail to resolve first: How exactly should one put tape into verbose mode when running in the browser? On the command line this can be trivially accomplished with a --verbose CLI flag. In a server-side process you can use a command line flag when starting the process or use ENV variables. With a browser side test, this isn't so easy since you may not have any real control over how the runtime gets executed/setup or what global context is available at runtime.
Also, should explain() be a binary on/off feature, or should it support some sort of log levels feature. e.g. only show me logging information of type performance or with importance above a level of 40
@Ovid any thoughts on diagnostic output levels versus a simple on/off switch?
—
Reply to this email directly or view it on GitHub.

@Ovid cool. That's what I figured, but since I haven't had access to an explain() method, it's best to hear from someone who has experience knowing how it's used.

@Raynos Any thoughts on how the explain flag should be turned on?

@malandrew

• TAP_EXPLAIN=1 as env variable.
• --explain as command line argument.
• ?explain=true as query string in browser
• window.TAP_EXPLAIN=1 as global in browser (approximation of env variable)
• #explain=true as hash section in browser
  -require('tape').explain = true as code driven togglable for node & browser.

I personally like TAP_EXPLAIN in env variable, window and ?explain=true in query string (browser).

Probably supporting require('tape').explain = true is not a bad idea.

@Raynos you, sir, are an officer and a gentleman. thank you. I'll open up a new pull request for explain(), but it will be dependent on diag() being merged in. I'll add each of those in separate commits so they can be cherry picked if there is one we decide we don't want.

Personally, my two favorites are your two favorites as well. The window.TAP_EXPLAIN would also need to work with global.TAP_EXPLAIN so that it works in node and in the browser. The --explain command line argument really is a function of the test runner and is out of scope for substack/tape, but not out of scope for isaacs/node-tap.

Is there any reason to support both the query string and the hash section? Seems like only one should be implemented for simplicity and that querystring is a better choice since it's semantically congruent with the intent of querystrings (whereas hashes were originally meant to be for anchors and AFAIK either don't support an X=Y syntax or at least was never meant to).

Agree that the require('tape').explain = true is not a bad idea, since that leaves someone the option for only turning on explain inline for a few assertions and then disabling it again. Should it just be a boolean or implemented as tape.enableExplain() and tape.disableExplain()?

For the ENV vars, is there a browserify "transform"/"plugin" that supports receiving an array of ENV variables that should be included in the final browserified script as a global? i.e. $ browserify --env=TAP_EXPLAIN,DEBUG ./my-file.js > bundle.js

@malandrew I prefer a tape.explain boolean because it doesnt add more methods to the interface and it's kind of a hackable implementation detail.

i.e. tape.explain = process.env.TAP_EXPLAIN || global.TAP_EXPLAIN || ... if you implement the explain toggle like that and then use if (tape.explain) in the test.explain() method, you get the feature that people can toggle it to something different without env variables / globals / uri strings.

I think doing both query & hash in the uri is overkill, query string is fine.

There is an https://github.com/hughsk/envify transform that inlines all process.env.foo

Do we still want this ? If so we can #shipit

@Raynos Yes! tap-parser supports diag so it would be good

👍 can we get this merged in?

It needs a rebase and conflicts resolved, or, the "allow edits" checkbox checked.

@isaacs are you maintaining this repo also?

@gabrielcsapo please don't ping arbitrary people unnecessarily; this PR can't be merged until the OP, @malandrew, updates it.

I was going to rebase the commit if @malandrew didn't respond. I was asking the question to Isaacs if this is being maintained any longer or if I should just move to tap. @ljharb

Yes, of course tape is being maintained, by multiple people.

I'd prefer the original poster be the one to do it, so we can modify this PR in-place instead of creating another one and polluting the git log.

I am not maintaining tape. All the tap-related things I do are under the tapjs github org.

@isaacs does that mean node-tape is being deprecated in favor of tapjs?

@gabrielcsapo No. Tape and node-tap are separate projects. Always have been.

@isaacs okay, but the output is based on the tap standard version 13 currently right? I was looking to see if tape would be updated to adhere to that spec.

@andrewdeandrade can you please check the "allow edits" checkbox on the RHS of the PR?

Closing out due to lack of activity.

I prefer to keep inactive PRs open, in case the author returns.